Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Stubs for rmw_get_publishers_info_by_topic and rmw_get_subscriptions_info_by_topic #377

Merged
merged 7 commits into from
Jan 14, 2020
Merged

Stubs for rmw_get_publishers_info_by_topic and rmw_get_subscriptions_info_by_topic #377

merged 7 commits into from
Jan 14, 2020

Conversation

jaisontj
Copy link
Contributor

@jaisontj jaisontj commented Oct 22, 2019 •
edited
Loading

NOTE: DO NOT MERGE until rmw #186 and rmw_implementation #72 are merged.

Leaving stubs for rmw_get_publishers_info_by_topic and rmw_get_subscriptions_info_by_topic as per discussion here

Related to - aws-roadmap#94

@jaisontj jaisontj marked this pull request as ready for review November 6, 2019 18:01
@jaisontj jaisontj mentioned this pull request Nov 6, 2019
Merged
@jaisontj jaisontj force-pushed the jaisontj/stub_rmw_get_topic_info branch from b7bed9f to c551d72 Compare November 13, 2019 23:34
prajakta-gokhale
prajakta-gokhale approved these changes Nov 13, 2019
View reviewed changes
Copy link

@prajakta-gokhale prajakta-gokhale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@jaisontj jaisontj force-pushed the jaisontj/stub_rmw_get_topic_info branch from afb8b3f to c6d7ae3 Compare November 18, 2019 21:16
@jaisontj
Copy link
Contributor Author

@ivanpauno

ivanpauno
ivanpauno reviewed Dec 10, 2019
View reviewed changes
Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment than here ros2/rmw_opensplice#289 (review).

rmw_connext_dynamic_cpp/src/functions.cpp Show resolved Hide resolved
rmw_connext_dynamic_cpp/src/functions.cpp Outdated Show resolved Hide resolved
rmw_connext_dynamic_cpp/src/functions.cpp Outdated Show resolved Hide resolved
rmw_connext_shared_cpp/src/topic_info.cpp Outdated Show resolved Hide resolved
rmw_connext_shared_cpp/src/topic_info.cpp Outdated Show resolved Hide resolved
@jaisontj jaisontj mentioned this pull request Dec 11, 2019
Closed
@jaisontj
Copy link
Contributor Author

Issue to track implementation of these stubs is here

@mm318 mm318 force-pushed the jaisontj/stub_rmw_get_topic_info branch from fed79ad to 0aee62d Compare December 11, 2019 20:01
@mm318
address PR comments
dde52b5 
Signed-off-by: Miaofei <[email protected]>
@mm318 mm318 force-pushed the jaisontj/stub_rmw_get_topic_info branch from 0aee62d to dde52b5 Compare December 11, 2019 20:01
hidmic
hidmic approved these changes Dec 17, 2019
View reviewed changes
Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM too

@prajakta-gokhale prajakta-gokhale mentioned this pull request Jan 3, 2020
Closed
wjwwood
wjwwood approved these changes Jan 8, 2020
View reviewed changes
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, with the understanding that these won't be left unimplemented for long since this is one of our two tier 1 rmw implementations.

rmw_connext_cpp/src/rmw_get_topic_info.cpp Outdated Show resolved Hide resolved
@ivanpauno ivanpauno merged commit 184f6d2 into ros2:master Jan 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mm318 mm318 mm318 left review comments

@prajakta-gokhale prajakta-gokhale prajakta-gokhale approved these changes

@ivanpauno ivanpauno ivanpauno approved these changes

@hidmic hidmic hidmic approved these changes

@wjwwood wjwwood wjwwood approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jaisontj @wjwwood @mm318 @hidmic @ivanpauno @prajakta-gokhale